Looks like I did not enable the "serialize" feature when running tests locally and got confused by the double negative.

I.e. not_in should say

yes, ns1:tag does not appear in [ some, namespace, ns1:tag ]

because the former is considered as tag and the latter is not yet supported

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.51%. Comparing base (29962e7) to head (745278a).
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #736      +/-   ##
==========================================
+ Coverage   61.48%   61.51%   +0.03%     
==========================================
  Files          38       38              
  Lines       16213    16224      +11     
==========================================
+ Hits         9968     9981      +13     
+ Misses       6245     6243       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hey, thank you for the review. My case is probably closer to #212, but that was closed as a duplicate of #347.

I agree that a regression test would be useful so I'll add one when I find time for it. This change doesn't do anything to fix serialization, but it tries to improve upon the current behavior of only considering the local part of a QName for deserialization.

My actual case is as follows:

<ns1:parent>
    <ns1:field />
    <ns1:one />
</ns1:parent>

with the following data model

#[derive(Deserialize)]
#[serde[renameAll = "camelCase")]
enum Choice {
  One,
  Two
}

#[derive(Deserialize)]
struct Field;

#[derive(Deserialize)]
#[serde[renameAll = "camelCase")]
struct Parent {
  field: Field,
  #[serde(rename = "$value")]
  choice: Vec<Choice>,
}

Today, this will try to find "field" as a variant in "Choice" because the not_in && has_value_field case triggers when deserializing the struct.

With the change in this PR, it will correctly identify "field" as a key on the struct even for "ns1:field".

Renaming the field to "ns1:field" via serde does not work, which I guess is what #347 is about: The not_in would trigger but the QNameDeserializer will pick just the local part and fail to find the field so you get "missing field" errors.

I imagine that adding a toggle in Deserializer (use local name vs full name for decode_name) could be a backwards compatible change to resolve #347.

I checked, and this PR does not solve #347, but solve #683, because it actually does the same thing as #702.

Unfortunately, #683 and #347 seems to contradict each other. #347 requires to consider prefix part of a tag. Well, it should be possible to fix both issues, but this simple fix just solve the #683 case, but makes us a little more far from solving #347 (in particular, this change makes it impossible to deal with that example).

I would prefer to investigate possibilities to solve both cases (#683 and #347).

cc @leftmostcat

I would prefer to investigate possibilities to solve both cases (#683 and #347).

Would it make sense to, unless the field is named $text or $value, parse the field name as an XML name and when deserializing, only compare local name to local name?

FWIW this question sent me down a rabbit hole of "canonical" encodings of a namespaced name and/or QName into a single string in XML and whether it's appropriate to use that as a field identifier. Turns out that URI isn't the answer to that, instead the {http://example.com/namespace}element syntax seems to be established for it. If we assume the @attribute syntax is indeed inspired by XPath, it might be reasonable to explore Q{http://example.com/namespace}element from XPath 3.0 for namespace support.

Furthermore, I think it would be wrong to use a serde field name of ns1:element unless going for a pure, pre https://www.w3.org/TR/xml-names/ tag/attribute name handling mode. QNames are defined to be equivalent if their namespace and local name match, so attaching meaning to a specific prefix string value via making it part of the serde property would prevent its use for certain well-formed documents.

In my mind, #347 is more about making sure the roundtrip works without different configuration for serialization and deserialization, regardless of any particular style of #[serde(...)] attributes. Suggesting that it's valid to serialize with an arbitrary namespace prefix by annotating the field with a literal prefix ns1: not tied to some xmlns:ns1 machinery is, in my understanding of XML Namespaces, a flawed assumption.

All that to say: After all that reading, I still think using the local part only is the correct first step toward full namespace support.

Furthermore, I think it would be wrong to use a serde field name of ns1:element unless going for a pure, pre https://www.w3.org/TR/xml-names/ tag/attribute name handling mode.

Yes, I agree with that.

All that to say: After all that reading, I still think using the local part only is the correct first step toward full namespace support.

Yes, that's what I say in my previous comment.

Furthermore, I think it would be wrong to use a serde field name of ns1:element unless going for a pure, pre https://www.w3.org/TR/xml-names/ tag/attribute name handling mode.

Yes, I agree with that.

All that to say: After all that reading, I still think using the local part only is the correct first step toward full namespace support.

Yes, that's what I say in my previous comment.

In that case, would it be possible to merge this PR as-is and do improvements to namespace handling as follow ups? Right now this bug (QNameDeserializer and not_in are inconsistent) severely reduces the usability of $value.

Yes, I'll combine your PR and #702, add tests and changelog. I'll plan to release new version soon, maybe this weekend. Unfortunately too many job at work and Kreia in KotOR2 is so intriguing. Anyway, thanks for your work!

Closed by #702